refactor(audience): identity and consent strings, SDK test pinning (SDK-277)#729
Open
ImmutableJeffrey wants to merge 12 commits intochore/sdk-272-pr-4-fixtures-and-sample-polishfrom
Open
Conversation
3044693 to
a5e7bd1
Compare
d5c1917 to
33aa3e6
Compare
a5e7bd1 to
76424cd
Compare
33aa3e6 to
01cc27c
Compare
76424cd to
08f5caf
Compare
01cc27c to
e513ad1
Compare
08f5caf to
8245c1c
Compare
e513ad1 to
1eb56dd
Compare
8245c1c to
e0114c0
Compare
1eb56dd to
1a3834e
Compare
e0114c0 to
7d06189
Compare
1a3834e to
3bf92fe
Compare
7d06189 to
4c052b9
Compare
3bf92fe to
b05092a
Compare
…-format strings The wire-format strings emitted under MessageFields.IdentityType and the ConsentLevel display strings used to live as inline literals in three places: the SDK's switch maps in IdentityType.cs / ConsentLevel.cs, the [TestCase] attributes in IdentityTypeTests / ConsentLevelTests, and the sample-app's SampleAppUi.Consent dropdown / status-cell strings. Adds: - IdentityTypeWireFormat (Passport, Steam, Epic, Google, Apple, Discord, Email, Custom) in IdentityType.cs - ConsentLevelWireFormat (None, Anonymous, Full) in ConsentLevel.cs Migrations: - IdentityTypeExtensions.ToLowercaseString and ParseLowercaseString switches now reference IdentityTypeWireFormat. - ConsentLevelExtensions.ToLowercaseString switch references ConsentLevelWireFormat. - IdentityTypeTests / ConsentLevelTests [TestCase] attributes reference the consts. - SampleAppUi.Consent constants delegate to ConsentLevelWireFormat. Centralising loses the implicit wire-format pinning the duplicated [TestCase] strings used to provide (a typo in the SDK switch would no longer be caught by the test, since both reference the same const). Restored explicitly via two new tests: IdentityTypeWireFormat_PinsExactStringValues and ConsentLevelWireFormat_PinsExactStringValues, which assert each const equals its expected wire string. A backend contract break or accidental rename now fails at the pinning test rather than slipping through. Per the user's "everything random goes in a constant" stance. Follow-up to SDK-272 (centralisation of duplicated literals).
…ield references
TypedEventTests asserted that argument-validation error messages
contained the C# property name being checked: Does.Contain("Status"),
Does.Contain("Flow"), Does.Contain("Currency") (twice),
Does.Contain("Amount"), Does.Contain("Value").
Each inline string mirrored a Progression / Resource / Purchase
property name. A property rename in the SDK (Resource.Currency to
Resource.CurrencyCode, say) would break the message wording but
pass the test, since "Currency" was matched as a substring even
when the SDK no longer mentioned it.
Replaces six inline strings with nameof(Progression.Status),
nameof(Resource.Flow), nameof(Resource.Currency), nameof(Resource.Amount),
nameof(Purchase.Currency), nameof(Purchase.Value). Each now tracks
the property name at compile time; renaming the property either
updates the test automatically or fails to compile.
Per the user's "everything random goes in a constant" stance.
Follow-up to SDK-272 (centralisation of duplicated literals).
LogTests had inline literals for the Log.Debug / Log.Warn inputs
("silent", "hello", "something off"), the prefix substring asserted
on emitted lines ("[ImmutableAudience]"), and the warn marker
("WARN").
Adds four file-local consts (SilentDebugInput, EnabledDebugInput,
WarnInput, WarnMarker) at the top of the fixture and migrates the
six call sites. The prefix assertion now references Log.Prefix, the
SDK's existing const for the same string, so a SDK-side prefix
rename automatically propagates.
The "WARN" substring is kept as a local marker rather than slicing
Log.WarnPrefix; "[ImmutableAudience] WARN:" includes punctuation
the assertion does not require. A direct WARN constant matches the
test's actual intent ("the warn pipeline includes WARN somewhere").
Per the user's "everything random goes in a constant" stance.
Follow-up to SDK-272 (centralisation of duplicated literals).
ImmutableAudienceTests was using inline strings to assert against captured log lines and queue file
contents:
- Has.Some.Contains("Dropping") for the dropped-event marker that Track / Identify / Alias warns
share
- "\"purchase\"", "\"identify\"", "\"alias\"" for queue-file content checks (the wire-format
envelope's "type" / "eventName" fields)
Adds AudienceLogs.DroppingMarker = "Dropping" so the marker has a single source of truth and the
test references it directly.
Migrates queue-file content checks to interpolate against the SDK's existing EventNames.Purchase,
MessageTypes.Identify, and MessageTypes.Alias constants so a backend rename to those wire strings
would touch one place rather than scatter across the test.
Per the user's "everything random goes in a constant" stance.
Follow-up to SDK-272 (centralisation of duplicated literals).
…ic user ID fixtures ImmutableAudienceTests had four inline ID fixtures around the identity / alias write tests: - "76561198012345": real-shape Steam community 64-bit ID, used as the Identify input AND as the queue-content assertion subject - "user1": minimal generic userId for the discarded-identify test - "steam123": generic Steam ID for the alias-write test, used as both Alias input and queue-content assertion subject - "user_456": generic Passport ID for the alias-write test Adds SteamId64, GenericUserSingleId, SteamId, PassportId to TestFixtures and migrates the six call sites. The SteamId64 / SteamId input-assertion pairs were the worst offenders since a typo on one side would silently pass on the other. Per the user's "everything random goes in a constant" stance. Follow-up to SDK-272 (centralisation of duplicated literals).
…StoreTests inline JSON keys
SessionTests had "track explode" inline four times as the
InvalidOperationException message thrown by ThrowingTrack delegates
across four session lifecycle scenarios. Adds a TrackExplodeMessage
file-local const at the top of the fixture and migrates all four.
DiskStoreTests built a Purchase-shaped JSON payload by hand-rolling
the wire format with inline keys ("type", "eventName", "anonymousId",
"userId", "properties", "currency", "value") and inline values
("track", "purchase", "USD", "a"). Replaces those with the SDK's
existing constants (MessageFields.*, MessageTypes.Track,
EventNames.Purchase, EventPropertyKeys.Currency / .Value) and the
TestFixtures fixtures already centralised (UsdCurrency,
TestEventNames.PlaceholderA).
The "u" userId placeholder is left inline; centralising a single
one-char placeholder would add more noise than it removes.
Per the user's "everything random goes in a constant" stance.
Follow-up to SDK-272 (centralisation of duplicated literals).
Renames TestFixtures.DistributionPlatformSteamCased and DistributionPlatformSteamUppercase to
neutral SteamPascalCase and SteamUpperCase. The same "Steam" / "STEAM" string fixtures are relevant
to two test families: ImmutableAudienceTests' DistributionPlatform lowercase normalisation, and
IdentityTypeTests' ParseLowercaseString case-insensitive matching. The new names drop the
family-specific prefix.
Adds:
- PassportPascalCase ("Passport") for the IdentityType mixed-case fixture batch.
- SteamSuffixed ("steamX") for the steam-prefix-but-not-exact-match fixture in the Custom-fallback
test.
Migrates references:
- ImmutableAudienceTests:1127 / 1138 (rename only)
- IdentityTypeTests:55-57 (ParseLowercaseString_AcceptsMixedCase cases)
- IdentityTypeTests:66 ([TestCase("steamX")])
- ImmutableAudienceTests:398 (missed inline "user1", now GenericUserSingleId from the earlier
centralisation pass)
Per the user's "everything random goes in a constant" stance.
Follow-up to SDK-272 (centralisation of duplicated literals).
…ort, stress, and offline tests Three test files had inline publishable-key fixtures with no intra-test reason for diverging from the canonical TestDefaults.PublishableKey: - ThreadSafetyStressTests:48 used "pk_imapik-test-stress" - OfflineResilienceTests:47 used "pk_imapik-test-key" Both fall under "any test-prefix key, content does not matter". Migrated to TestDefaults.PublishableKey. HttpTransportTests:162 used "pk_imapik-prodkey" specifically to verify a non-test-prefix key resolves to ProductionBaseUrl. The test cannot use TestDefaults.PublishableKey since that one IS test-prefixed. Added a file-local ProdPublishableKey const alongside the existing response-body fixtures and migrated the one usage. The const carries a comment explaining the prefix constraint. Per the user's "everything random goes in a constant" stance. Follow-up to SDK-272 (centralisation of duplicated literals).
HttpTransportTests had "Retry-After" inline three times (across the delta-seconds, http-date, and past-date 429 tests) and ConsentSyncTests had it once in CapturingHandler.SendAsync. The string is the RFC 7231 standard header name; the SDK reads it via the typed response.Headers.RetryAfter accessor, but the mocks set it by name. Adds a file-local RetryAfterHeader const at the top of each fixture (matching the existing publishable-key / response-body file-local const pattern) and migrates all four sites. Per the user's "everything random goes in a constant" stance. Follow-up to SDK-272 (centralisation of duplicated literals).
…d values JsonTests had inline scenario fixtures across nineteen tests: - "flag" (Bool true / false serialise pair) - "n" (Int / Long serialise) - "x" (Null serialise) - "v" (eight float / double NaN, Infinity, normal-range, large / small exponent tests; reused as the diamond test's inner value) - "items" (List serialise) - "level", "score", "perfect", "tags" (RealisticEventPayload nested properties keys) - "fast", "clean" (tags array element values) - "next" (deep-nesting MaxDepth guard) - "self" (cycle-detection self-reference) - "cycle" / "nesting exceeds" (FormatException message markers) - "k", "a", "b" (diamond shared-child scenario) Adds a file-local const block at the top of the fixture grouping the literals by scenario (Bool / Numeric / Null / V / Array, RealisticPayload, cycle / depth, diamond) and migrates every test that consumed them. Where an SDK / fixtures const already covered the same string (TestEventNames.PlaceholderA for "a", TestEventNames.LevelComplete for "level_complete"), the test now references that instead. Per the user's "everything random goes in a constant" stance. Follow-up to SDK-272 (centralisation of duplicated literals).
…malformed inputs
JsonReaderTests had inline scenario fixtures across the deserialise
tests:
- "small" / "big" (IntAndLong)
- "t" / "f" / "n" (BoolAndNull)
- "arr" / "two" (Array)
- "abc" (RoundTripViaSerializer anonymousId)
- "76561198012345" (RoundTripViaSerializer userId; same string also
used in ImmutableAudienceTests' Steam Identify test)
- "{not valid}" / "{\"a\":}" / "{\"a\":\"unterminated" (three
malformed inputs in MalformedThrows)
Adds a file-local const block at the top of the fixture grouping the
keys, the array-element value, the anonymousId placeholder, and the
three malformed inputs. Each test now interpolates the encoded JSON
form from its key consts so a key rename touches one place. The
"76561198012345" duplication is removed by referencing
TestFixtures.SteamId64 (already centralised).
Per the user's "everything random goes in a constant" stance.
Follow-up to SDK-272 (centralisation of duplicated literals).
EventQueueTests' local Msg helper labelled its dict's event-name
field with an inline "event" key. The string is distinct from the
SDK's wire-format MessageFields.EventName ("eventName"); EventQueue
accepts arbitrary dicts so the test could pick anything. Inline made
this implicit.
Adds a file-local MsgEventKey const at the top of the fixture with a
comment noting the test-scaffolding role and migrates the one usage.
Per the user's "everything random goes in a constant" stance.
Follow-up to SDK-272 (centralisation of duplicated literals).
4c052b9 to
e2d1ce2
Compare
b05092a to
9909f49
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
IdentityTypeandConsentLevelwire-format strings intoIdentityTypeWireFormatandConsentLevelWireFormat; the SDK switches and the sample-app dropdown read from one source.nameof().LogTestsinputs and routes log-prefix references throughLog.Prefix.EventQueueTestsassertions through SDK constants for the log dropping marker,MessageTypes, andEventNames.TestFixtureswith a real-shape Steam community ID, generic Steam / Passport IDs, and a generic single-user fixture.SessionTestssabotage exception message andDiskStoreTestsinline JSON keys.DistributionPlatformandIdentityTypetests.ConsentSyncTestsandHttpTransportTests.JsonTestsper-scenario fixture keys and values.JsonReaderTestsscenario fixtures and malformed inputs.EventQueueTestsMsghelper key.OnErrorcallback contract unchanged.Linear: SDK-277